Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refine Digest #1779

Closed
wants to merge 1 commit into from
Closed

Refine Digest #1779

wants to merge 1 commit into from

Conversation

ksss
Copy link
Collaborator

@ksss ksss commented Mar 27, 2024

  • Should be return bot for methods of abstruct class. Use interface instead.
  • The number of arguments of Digest::Class methods cannot be determined because they are delegated to the inherited initialize.
  • Fix some types.

Good points

  • Become a more accurate signatures.

Bad points

  • Comments are effectively lost because annotations cannot be automatically written on the interface.

@ksss
Refine Digest
7886c29 
- Should be return bot for methods of abstruct class. Use interface instead.
- The number of arguments of Digest::Class methods cannot be determined because they are delegated to the inherited initialize.
- Fix some types.
@soutaro
Copy link
Member

soutaro commented May 29, 2024

Having -> bot types doesn't look a good idea, especially for Digest::Base. We may want to have Digest::Base type which might be one of the sub classes instances, and it all method calls will result in bot.

@ksss
Copy link
Collaborator Author

ksss commented May 29, 2024

I understand. I would like to modify it with a different approach. I will close this PR.

@ksss ksss closed this May 29, 2024
@ksss ksss deleted the digest branch May 29, 2024 12:26
@ksss ksss mentioned this pull request Jun 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ksss @soutaro